Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jun 16, 2025

@sbc100 sbc100 requested a review from tlively June 16, 2025 18:48
@sbc100
Copy link
Collaborator Author

sbc100 commented Jun 16, 2025

is it me or are these fallback functions kind of dangerous / wrong?

@sbc100 sbc100 requested a review from kripken June 16, 2025 18:53
@tlively
Copy link
Member

tlively commented Jun 16, 2025

What looks wrong about them? They look like reasonable single-threaded implementations of compare-and-swap and fetch-add.

@sbc100
Copy link
Collaborator Author

sbc100 commented Jun 16, 2025

Right, but as we have shown by the need for this PR the COMPILER_RT_HAS_ATOMICS might not correspond 1-to-1 with single-threaded-only.

In this case we were building without any macros and it was assuming incorrectly that single-theaded version could be substituted (even on multi-threaded builds). Perhaps it would be safer to invert the macro and have COMPILER_RT_SINGLE_THREADED_ONLY or some such.

@sbc100 sbc100 merged commit e1692b4 into emscripten-core:main Jun 16, 2025
30 checks passed
@sbc100 sbc100 deleted the COMPILER_RT_HAS_ATOMICS branch June 16, 2025 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants